-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Added Audio to FastMCP #1130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Added Audio to FastMCP #1130
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dragonier23 !
Looks good / in line w/ Image support! Suggesting some more tests before we can merge :-)
content = result.content[0] | ||
assert isinstance(content, AudioContent) | ||
assert content.type == "audio" | ||
assert content.mimeType == "audio/wav" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add something to exercice the suffix-based mime type detection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added a test to test the mimetypes, do let me know if this is what you are looking for!
@pytest.mark.anyio | ||
async def test_tool_mixed_list_with_audio(self, tmp_path: Path): | ||
"""Test that lists containing Audio objects and other types are handled | ||
correctly""" | ||
# Create a test audio | ||
audio_path = tmp_path / "test.wav" | ||
audio_path.write_bytes(b"test audio data") | ||
|
||
def mixed_list_fn() -> list: | ||
return [ | ||
"text message", | ||
Audio(audio_path), | ||
{"key": "value"}, | ||
TextContent(type="text", text="direct content"), | ||
] | ||
|
||
mcp = FastMCP() | ||
mcp.add_tool(mixed_list_fn) | ||
async with client_session(mcp._mcp_server) as client: | ||
result = await client.call_tool("mixed_list_fn", {}) | ||
assert len(result.content) == 4 | ||
# Check text conversion | ||
content1 = result.content[0] | ||
assert isinstance(content1, TextContent) | ||
assert content1.text == "text message" | ||
# Check audio conversion | ||
content2 = result.content[1] | ||
assert isinstance(content2, AudioContent) | ||
assert content2.mimeType == "audio/wav" | ||
assert base64.b64decode(content2.data) == b"test audio data" | ||
# Check dict conversion | ||
content3 = result.content[2] | ||
assert isinstance(content3, TextContent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this test and add Audio on the above please.
test_cases = [ | ||
("test.wav", "audio/wav"), | ||
("test.mp3", "audio/mpeg"), | ||
("test.ogg", "audio/ogg"), | ||
("test.flac", "audio/flac"), | ||
("test.aac", "audio/aac"), | ||
("test.m4a", "audio/mp4"), | ||
("test.unknown", "application/octet-stream"), # Unknown extension fallback | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use pytest.mark.parametrize
, please.
def __init__( | ||
self, | ||
path: str | Path | None = None, | ||
data: bytes | None = None, | ||
format: str | None = None, | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use @typing.override
.
if path is None and data is None: | ||
raise ValueError("Either path or data must be provided") | ||
if path is not None and data is not None: | ||
raise ValueError("Only one of path or data can be provided") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can do bool(path) ^ bool(data)
, and have a meaningful message: "Either path or data can be provided".
Motivation and Context
Adding Audio in FastMCP that can be used to send audio content back. Related to Issue Closes: #952
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context